Avoid updating the registry too frequently

This logic is based off the precision of the registry source's id as well as the
dependencies being passed in. More info can be found in the comments.

diff --git a/src/cargo/ops/cargo_generate_lockfile.rs b/src/cargo/ops/cargo_generate_lockfile.rs
index c318b8db9711a4b10d593a586b23d4263ce2c4f0..059e9064c16268b6b161d03c72187243d7df033a 100644 (file)
--- a/src/cargo/ops/cargo_generate_lockfile.rs
+++ b/src/cargo/ops/cargo_generate_lockfile.rs
@@ -49,7 +49,6 @@ pub fn update_lockfile(manifest_path: &Path,
     let mut config = try!(Config::new(opts.shell, None, None));
     let mut registry = PackageRegistry::new(&mut config);
     let mut to_avoid = HashSet::new();
-    let mut previous = Some(&previous_resolve);
 
     match opts.to_update {
         Some(name) => {
@@ -69,13 +68,13 @@ pub fn update_lockfile(manifest_path: &Path,
                 }
             }
         }
-        None => { previous = None; }
+        None => to_avoid.extend(previous_resolve.iter()),
     }
 
     let resolve = try!(ops::resolve_with_previous(&mut registry,
                                                   &package,
                                                   resolver::ResolveEverything,
-                                                  previous,
+                                                  Some(&previous_resolve),
                                                   Some(&to_avoid)));
     try!(ops::write_pkg_lockfile(&package, &resolve));
     return Ok(());

diff --git a/src/cargo/ops/resolve.rs b/src/cargo/ops/resolve.rs
index 7f2a9fd21762586751e11feb3db7d782263f0e59..62b6e9f3f352d7f1a93edd37b402e239b576c243 100644 (file)
--- a/src/cargo/ops/resolve.rs
+++ b/src/cargo/ops/resolve.rs
@@ -87,7 +87,7 @@ pub fn resolve_with_previous<'a>(registry: &mut PackageRegistry,
 
     let mut resolved = try!(resolver::resolve(&summary, method, registry));
     match previous {
-        Some(r) => resolved.copy_metadata(previous),
+        Some(r) => resolved.copy_metadata(r),
         None => {}
     }
     return Ok(resolved);

diff --git a/src/cargo/sources/registry.rs b/src/cargo/sources/registry.rs
index ad21312be06fed0a9a5110df39600ff586b1516b..f989559c3c1b176427af6e2182c93b84bd0de479 100644 (file)
--- a/src/cargo/sources/registry.rs
+++ b/src/cargo/sources/registry.rs
@@ -189,6 +189,7 @@ pub struct RegistrySource<'a, 'b:'a> {
     sources: Vec<PathSource>,
     hashes: HashMap<(String, String), String>, // (name, vers) => cksum
     cache: HashMap<String, Vec<(Summary, bool)>>,
+    updated: bool,
 }
 
 #[deriving(Decodable)]
@@ -239,6 +240,7 @@ impl<'a, 'b> RegistrySource<'a, 'b> {
             sources: Vec::new(),
             hashes: HashMap::new(),
             cache: HashMap::new(),
+            updated: false,
         }
     }
 
@@ -420,20 +422,11 @@ impl<'a, 'b> RegistrySource<'a, 'b> {
               .default_features(default_features)
               .features(features))
     }
-}
 
-impl<'a, 'b> Registry for RegistrySource<'a, 'b> {
-    fn query(&mut self, dep: &Dependency) -> CargoResult<Vec<Summary>> {
-        let summaries = try!(self.summaries(dep.get_name()));
-        let mut summaries = summaries.iter().filter(|&&(_, yanked)| {
-            dep.get_source_id().get_precise().is_some() || !yanked
-        }).map(|&(ref s, _)| s.clone()).collect::<Vec<_>>();
-        summaries.query(dep)
-    }
-}
+    /// Actually perform network operations to update the registry
+    fn do_update(&mut self) -> CargoResult<()> {
+        if self.updated { return Ok(()) }
 
-impl<'a, 'b> Source for RegistrySource<'a, 'b> {
-    fn update(&mut self) -> CargoResult<()> {
         try!(self.config.shell().status("Updating",
              format!("registry `{}`", self.source_id.get_url())));
         let repo = try!(self.open());
@@ -451,6 +444,41 @@ impl<'a, 'b> Source for RegistrySource<'a, 'b> {
         log!(5, "[{}] updating to rev {}", self.source_id, oid);
         let object = try!(repo.find_object(oid, None));
         try!(repo.reset(&object, git2::Hard, None, None));
+        self.updated = true;
+        self.cache.clear();
+        Ok(())
+    }
+}
+
+impl<'a, 'b> Registry for RegistrySource<'a, 'b> {
+    fn query(&mut self, dep: &Dependency) -> CargoResult<Vec<Summary>> {
+        // If this is a precise dependency, then it came from a lockfile and in
+        // theory the registry is known to contain this version. If, however, we
+        // come back with no summaries, then our registry may need to be
+        // updated, so we fall back to performing a lazy update.
+        if dep.get_source_id().get_precise().is_some() &&
+           try!(self.summaries(dep.get_name())).len() == 0 {
+            try!(self.do_update());
+        }
+
+        let summaries = try!(self.summaries(dep.get_name()));
+        let mut summaries = summaries.iter().filter(|&&(_, yanked)| {
+            dep.get_source_id().get_precise().is_some() || !yanked
+        }).map(|&(ref s, _)| s.clone()).collect::<Vec<_>>();
+        summaries.query(dep)
+    }
+}
+
+impl<'a, 'b> Source for RegistrySource<'a, 'b> {
+    fn update(&mut self) -> CargoResult<()> {
+        // If we have an imprecise version then we don't know what we're going
+        // to look for, so we always atempt to perform an update here.
+        //
+        // If we have a precise version, then we'll update lazily during the
+        // querying phase.
+        if self.source_id.get_precise().is_none() {
+            try!(self.do_update());
+        }
         Ok(())
     }
 

diff --git a/tests/test_cargo_registry.rs b/tests/test_cargo_registry.rs
index 27e34d6690fad6362d09d6c7b771deb52b1f1237..596764b26a3816410f30ccd8e66cea59b9a2dd09 100644 (file)
--- a/tests/test_cargo_registry.rs
+++ b/tests/test_cargo_registry.rs
@@ -2,7 +2,7 @@ use std::io::{fs, File};
 
 use support::{project, execs, cargo_dir};
 use support::{UPDATING, DOWNLOADING, COMPILING, PACKAGING, VERIFYING};
-use support::paths::PathExt;
+use support::paths::{mod, PathExt};
 use support::registry as r;
 
 use hamcrest::assert_that;
@@ -249,9 +249,7 @@ test!(lockfile_locks {
     r::mock_pkg("bar", "0.0.2", []);
 
     assert_that(p.process(cargo_dir().join("cargo")).arg("build"),
-                execs().with_status(0).with_stdout(format!("\
-{updating} registry `[..]`
-", updating = UPDATING).as_slice()));
+                execs().with_status(0).with_stdout(""));
 })
 
 test!(lockfile_locks_transitively {
@@ -287,9 +285,7 @@ test!(lockfile_locks_transitively {
     r::mock_pkg("bar", "0.0.2", [("baz", "*")]);
 
     assert_that(p.process(cargo_dir().join("cargo")).arg("build"),
-                execs().with_status(0).with_stdout(format!("\
-{updating} registry `[..]`
-", updating = UPDATING).as_slice()));
+                execs().with_status(0).with_stdout(""));
 })
 
 test!(yanks_are_not_used {
@@ -373,9 +369,7 @@ test!(yanks_in_lockfiles_are_ok {
     r::mock_pkg_yank("bar", "0.0.1", [], true);
 
     assert_that(p.process(cargo_dir().join("cargo")).arg("build"),
-                execs().with_status(0).with_stdout(format!("\
-{updating} registry `[..]`
-", updating = UPDATING).as_slice()));
+                execs().with_status(0).with_stdout(""));
 
     assert_that(p.process(cargo_dir().join("cargo")).arg("update"),
                 execs().with_status(101).with_stderr("\
@@ -384,3 +378,65 @@ location searched: the package registry
 version required: *
 "));
 })
+
+test!(update_with_lockfile_if_packages_missing {
+    let p = project("foo")
+        .file("Cargo.toml", r#"
+            [project]
+            name = "foo"
+            version = "0.0.1"
+            authors = []
+
+            [dependencies]
+            bar = "*"
+        "#)
+        .file("src/main.rs", "fn main() {}");
+    p.build();
+
+    r::mock_pkg("bar", "0.0.1", []);
+    assert_that(p.process(cargo_dir().join("cargo")).arg("build"),
+                execs().with_status(0));
+    p.root().move_into_the_past().unwrap();
+
+    fs::rmdir_recursive(&paths::home().join(".cargo/registry")).unwrap();
+    assert_that(p.process(cargo_dir().join("cargo")).arg("build"),
+                execs().with_status(0).with_stdout(format!("\
+{updating} registry `[..]`
+{downloading} bar v0.0.1 (the package registry)
+", updating = UPDATING, downloading = DOWNLOADING).as_slice()));
+})
+
+test!(update_lockfile {
+    let p = project("foo")
+        .file("Cargo.toml", r#"
+            [project]
+            name = "foo"
+            version = "0.0.1"
+            authors = []
+
+            [dependencies]
+            bar = "*"
+        "#)
+        .file("src/main.rs", "fn main() {}");
+    p.build();
+
+    r::mock_pkg("bar", "0.0.1", []);
+    assert_that(p.process(cargo_dir().join("cargo")).arg("build"),
+                execs().with_status(0));
+
+    r::mock_pkg("bar", "0.0.2", []);
+    fs::rmdir_recursive(&paths::home().join(".cargo/registry")).unwrap();
+    assert_that(p.process(cargo_dir().join("cargo")).arg("update")
+                 .arg("-p").arg("bar"),
+                execs().with_status(0).with_stdout(format!("\
+{updating} registry `[..]`
+", updating = UPDATING).as_slice()));
+
+    assert_that(p.process(cargo_dir().join("cargo")).arg("build"),
+                execs().with_status(0).with_stdout(format!("\
+{downloading} [..] v0.0.2 (the package registry)
+{compiling} bar v0.0.2 (the package registry)
+{compiling} foo v0.0.1 ({dir})
+", downloading = DOWNLOADING, compiling = COMPILING,
+   dir = p.url()).as_slice()));
+})